Skip to content

Glasgow | JAN ITP-2026|Tuan Nguyen | Sprint 1 | Programming-fundamentals #1140

Open
Jacknguyen4438 wants to merge 14 commits intoCodeYourFuture:mainfrom
Jacknguyen4438:programming-funamental-sprint-1
Open

Glasgow | JAN ITP-2026|Tuan Nguyen | Sprint 1 | Programming-fundamentals #1140
Jacknguyen4438 wants to merge 14 commits intoCodeYourFuture:mainfrom
Jacknguyen4438:programming-funamental-sprint-1

Conversation

@Jacknguyen4438
Copy link

Self checklist

  • I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
  • My changes meet the requirements of the task
  • I have tested my changes
  • My changes follow the style guide

Changelist

I have finish all 3 section from sprint 1:

  1. Key-exercises.
  2. Mandatory-errors.
  3. Mandatory-interpret
    If there is some exercise or course from sprint 1 I need to fix and improve please let me know.

Questions

Yes I see that in sprint 3 I need to make multiple PR request. Could I do that in the same branch for sprint 3 or I need to make like sub branch from sprint 3 branch in order to submit?

@github-actions

This comment has been minimized.

@Jacknguyen4438 Jacknguyen4438 added 📅 Sprint 1 Assigned during Sprint 1 of this module Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Mar 3, 2026
@github-actions

This comment has been minimized.

@github-actions github-actions bot removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Mar 3, 2026
@github-actions

This comment has been minimized.

@Jacknguyen4438 Jacknguyen4438 changed the title Glasgow | Scotland JAN ITP-2026|Tuan Nguyen | Module-2-Sprint 1 | Programming-fundamentals Glasgow | JAN ITP-2026|Tuan Nguyen | Sprint 1 | Programming-fundamentals Mar 3, 2026
@Jacknguyen4438 Jacknguyen4438 added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Module-Structuring-And-Testing-Data The name of the module. labels Mar 3, 2026
Copy link
Contributor

@cjyuan cjyuan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed some minor inconsistency formatting in the code/comments.

Have you installed prettier VSCode extension and enabled formatting on save/paste on VSCode as recommended in
https://github.com/CodeYourFuture/Module-Structuring-and-Testing-Data/blob/main/readme.md

Comment on lines 14 to 16
const pence = paddedPenceNumberString
.substring(paddedPenceNumberString.length - 2)
.padEnd(2, "0");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional challenge question (no change required):

Could we expect this program to work as intended for any valid penceString if we deleted .padEnd(2, "0") from the code?
In other words, do we really need .padEnd(2, "0") in this script? Why?

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Mar 5, 2026
@Jacknguyen4438
Copy link
Author

@cjyuan Hello Yuan, thank you for the review you give me I am very grateful for the insight. I can see that there are many section I need to fix, so I would like to notify you that I have read the PR review and in the progress of fixing it. I understand that you are very busy with other PR as well and I was radio silent do to I am doing sprint 3 of module 2 at the moment. When I finish fixing and ready to be review I will mention you again thank you.


carPrice = Number(carPrice.replaceAll(",", ""));
priceAfterOneYear = Number(priceAfterOneYear.replaceAll("," ""));
priceAfterOneYear = Number(priceAfterOneYear.replaceAll(",", ""));
Copy link

@ykamal ykamal Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine, but the link below is worth looking into. it's an abstraction over this and will probably serve you better in the long run:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/parseInt

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ykamal Thank you for the link. I have read through and see that it indeed more organise then the current method I write in the code. I will try to used in my course work next time.

@Jacknguyen4438
Copy link
Author

Hello @cjyuan I have make change base on the review suggestion from both you and ykamal. I am ready for another PR review and please if I need to make further change please let me know

@Jacknguyen4438 Jacknguyen4438 added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Mar 22, 2026
/*
const slashIndex = filePath.indexOf("/");
const dir =filePath.slice(slashIndex + 0, 45) ;
const ext =filePath.slice(slashIndex + 49, 53);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why keep this code? Best practice is to remove unused code to keep our code clean. And doing so also helps make code review easier.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for point this out, I forgot to remove it when I fix the code. When you check again this line of code will be remove

Comment on lines +27 to +30
let dirDirectory = filePath.slice(filePath.indexOf("/"),filePath.lastIndexOf("/"));
let extDirectory = filePath.slice(filePath.lastIndexOf("/"));
console.log(`The dir part of the file is ${dirDirectory}.`);
console.log(`The ext part of the file is ${extDirectory}.`);
Copy link
Contributor

@cjyuan cjyuan Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suppose filePath is "/tmp/file.txt". Based on your understanding of the anatomy of a file path on lines 3-8, can you tell which part of the file path is considered the dir part and which part of it is considered the ext part?

When you run your code, does your code output the values you expected?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the feed back I have check again and see some part that I write incorrect. My answer will be include when I finish and commit sync to github.

@cjyuan cjyuan removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Mar 22, 2026
Comment on lines +21 to +23
let dirDirectory = filePath.slice(filePath.indexOf("/"),filePath.lastIndexOf("."));
let extDirectory = filePath.slice(filePath.lastIndexOf("."));
let baseDirectory = filePath.slice(filePath.lastIndexOf("/"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The value of the "dir" part is still not correct.

Note:

  • "dir" already stands for "directory"
  • "ext" stands for extension (or file extension); it is not a directory
  • "base" is also not necessary a directory
  • The value of "base' has already been extracted on line 14. The value you extracted on line 23 is not quite correct (because your code includes the '/' character in the base).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Module-Structuring-And-Testing-Data The name of the module. Reviewed Volunteer to add when completing a review with trainee action still to take. 📅 Sprint 1 Assigned during Sprint 1 of this module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants